-
Notifications
You must be signed in to change notification settings - Fork 66
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merge v3.2.4 release candidate for final release #966
base: develop
Are you sure you want to change the base?
Conversation
@@ -49,8 +49,7 @@ jobs: | |||
shell: bash -l {0} | |||
run: | | |||
conda install curl eigen | |||
conda install -c conda-forge hdf5=1.10.6 | |||
conda remove -y yaml-cpp |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we still consider this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this change makes sense - it fixes one problem, but there is now a new linking problem on Windows because we don't find the HDF5 library at link time. I haven't had time to figure out why.
.github/workflows/mac_build_test.yml
Outdated
@@ -47,7 +47,7 @@ jobs: | |||
- name: Initial setup | |||
shell: bash -l {0} | |||
run: | | |||
brew install eigen hdf5 | |||
brew install --force eigen hdf5 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the new HDF5 comes with a new pkgconf, which is causing the linking issue.
We could consider running:
brew link --overwrite pkgconf
or
brew reinstall pkg-config
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know enough about brew to know in which order this would come relative to the lines we already have?
It appears that only HDF5 version 1.10.6 works for windows builds at the moment. The latest version, 1.14.4, doesn't include essential files like
For now, I’ve set the HDF5 version to 1.10.6. I assume, the MOAB library needs updates to properly support newer versions of HDF5 for Windows builds. |
@gonuke, it seems yaml-cpp is a core dependency now. Please let me know how to address the gtest error. |
I think this is failing for Windows on a very specific issue: CMake found the HDF5 libraries successfully during the configuration step, but they are not being found at the linking step. I am not sure how best to address this in Windows. I know there has been some substantial overhaul for Windows build in MOAB on the |
As @ahnaf-tahmid-chowdhury points out, none of the 1.14.x conda-forge builds include |
I am not sure about this. I found that HDF5 1.14 does not include |
Is there any specific reason for using the conda version of HDF5 here? If we build it from source, I believe we may be able to overcome all the issues we are currently facing here. |
Also, we should set our CMake version to at least 3.12 to enable |
If it's straightforward to build HDF5, we could certainly do that. What I'm concerned about is that building HDF5 for Windows launches us on a path for a complete overhaul of our build infrastructure. |
Seems unlikely since it's been a year since they included |
Perhaps building hdf5 from source is the only option then |
could we temporary fix this with a simple copy ? and have both name for the lib ? This is not a good solution, but we could that way move forward :) |
On older libraries, they are not the same file - they at least have different sizes. |
localize git submodule stuff to only when BUILD_UWUW
This release candidate has been available for 7 weeks and there are no additional changes requested.
Requires merge to support finalization of the draft release.